From 5213606b5694c33f35141356a7062f761373f466 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Tue, 27 Jul 2010 16:58:51 +0100 Subject: [PATCH] libxl: signal caller if domain already destroyed on domain death event Currently libxl_event_get_domain_death_info returns 0 if the event was not a domain death event and 1 if it was but does not infom the user if someone else has already cleaned up the domain, which means the caller must replicate some of the logic from within libxl. Instead have the libxl_event_get_XXX_info functions required that the event is of the right type (the caller must have recently switched on event->type anyway). This allows the return codes to be used in an event specific way and we take advantage of this by returning an error from libxl_event_get_domain_death_info if the domain is not dying. Signed-off-by: Ian Campbell Committed-by: Ian Jackson --- tools/libxl/libxl.c | 66 +++++++++++++++++----------------------- tools/libxl/libxl.h | 17 +++++++++++ tools/libxl/xl_cmdimpl.c | 14 ++++++--- 3 files changed, 54 insertions(+), 43 deletions(-) diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index efeb7118f3..960931b2b0 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -704,52 +704,42 @@ int libxl_free_waiter(libxl_waiter *waiter) int libxl_event_get_domain_death_info(struct libxl_ctx *ctx, uint32_t domid, libxl_event *event, struct libxl_dominfo *info) { - int rc = 0, ret; - if (event && event->type == LIBXL_EVENT_DOMAIN_DEATH) { - ret = libxl_domain_info(ctx, info, domid); + if (libxl_domain_info(ctx, info, domid) < 0) + return 0; - if (ret == 0 && info->domid == domid) { - if (info->running || (!info->shutdown && !info->dying)) - goto out; - rc = 1; - goto out; - } - memset(info, 0, sizeof(*info)); - rc = 1; - goto out; - } -out: - return rc; + if (info->running || (!info->shutdown && !info->dying)) + return ERROR_INVAL; + + return 1; } int libxl_event_get_disk_eject_info(struct libxl_ctx *ctx, uint32_t domid, libxl_event *event, libxl_device_disk *disk) { - if (event && event->type == LIBXL_EVENT_DISK_EJECT) { - char *path; - char *backend; - char *value = libxl_xs_read(ctx, XBT_NULL, event->path); + char *path; + char *backend; + char *value; - if (!value || strcmp(value, "eject")) - return 0; + value = libxl_xs_read(ctx, XBT_NULL, event->path); - path = strdup(event->path); - path[strlen(path) - 6] = '\0'; - backend = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/backend", path)); + if (!value || strcmp(value, "eject")) + return 0; - disk->backend_domid = 0; - disk->domid = domid; - disk->physpath = NULL; - disk->phystype = 0; - /* this value is returned to the user: do not free right away */ - disk->virtpath = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/dev", backend)); - disk->unpluggable = 1; - disk->readwrite = 0; - disk->is_cdrom = 1; - - free(path); - return 1; - } - return 0; + path = strdup(event->path); + path[strlen(path) - 6] = '\0'; + backend = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/backend", path)); + + disk->backend_domid = 0; + disk->domid = domid; + disk->physpath = NULL; + disk->phystype = 0; + /* this value is returned to the user: do not free right away */ + disk->virtpath = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/dev", backend)); + disk->unpluggable = 1; + disk->readwrite = 0; + disk->is_cdrom = 1; + + free(path); + return 1; } static int libxl_destroy_device_model(struct libxl_ctx *ctx, uint32_t domid) diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index 950189c188..387fa5c821 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -393,7 +393,24 @@ int libxl_stop_waiting(struct libxl_ctx *ctx, libxl_waiter *waiter); int libxl_free_event(libxl_event *event); int libxl_free_waiter(libxl_waiter *waiter); +/* + * Returns: + * - 0 if the domain is dead but there is no cleanup to be done. e.g + * because someone else has already done it. + * - 1 if the domain is dead and there is cleanup to be done. + * + * Can return error if the domain exists and is still running. + * + * *info will contain valid domain state iff 1 is returned. In + * particular if 1 is returned then info->shutdown_reason is + * guaranteed to be valid since by definition the domain is + * (shutdown||dying)) + */ int libxl_event_get_domain_death_info(struct libxl_ctx *ctx, uint32_t domid, libxl_event *event, struct libxl_dominfo *info); + +/* + * Returns true and fills *disk if the caller should eject the disk + */ int libxl_event_get_disk_eject_info(struct libxl_ctx *ctx, uint32_t domid, libxl_event *event, libxl_device_disk *disk); int libxl_domain_rename(struct libxl_ctx *ctx, uint32_t domid, diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 8cae2597e9..ec8f25f18a 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -1338,7 +1338,6 @@ start: struct libxl_dominfo info; libxl_event event; libxl_device_disk disk; - memset(&info, 0x00, sizeof(xc_domaininfo_t)); FD_ZERO(&rfds); FD_SET(fd, &rfds); @@ -1349,12 +1348,17 @@ start: libxl_get_event(&ctx, &event); switch (event.type) { case LIBXL_EVENT_DOMAIN_DEATH: - if (libxl_event_get_domain_death_info(&ctx, domid, &event, &info)) { - LOG("Domain %d is dead", domid); - if (info.dying || (info.shutdown && info.shutdown_reason != SHUTDOWN_suspend)) { + ret = libxl_event_get_domain_death_info(&ctx, domid, &event, &info); + + if (ret < 0) continue; + + LOG("Domain %d is dead", domid); + + if (ret) { + if (info.shutdown_reason != SHUTDOWN_suspend) { LOG("Domain %d needs to be clean: destroying the domain", domid); libxl_domain_destroy(&ctx, domid, 0); - if (info.shutdown && info.shutdown_reason == SHUTDOWN_reboot) { + if (info.shutdown_reason == SHUTDOWN_reboot) { libxl_free_waiter(w1); libxl_free_waiter(w2); free(w1); -- 2.30.2